Repository navigation
feat: better grpc handling - #1104
Conversation
GRPCResponse._handle_grpc_response returned early for any non-OK response, which made both of the checks below it unreachable: expecting a 'body' alongside a non-OK status silently passed. Move the 'body specified but expected status is not OK' check above the early return, and fetch the response message inside a try/except so a failed call reports that the expected body could not be checked instead of being skipped. The grpc unit tests now assert the exception each spec is expected to raise and the status the server actually returned, rather than calling pytest.xfail() before doing any work.
The 'details' key of a grpc response was unusable: the code compared it against the details string of the response, but the schema said it was an object, so the only value it could ever have matched was rejected before the test ran. Split the two things which can be called the 'details' of a response: - 'error_message' checks the string the server passed to set_details or abort. It goes through the normal matching, so type sentinels like !re_search can be used on it. - 'details' checks the error details attached to the response - the messages packed into the google.rpc.Status in the trailing metadata, each converted to a mapping including its '@type'. As documented in https://grpc.io/docs/guides/error/ this is a Google convention for using grpc with protobuf rather than part of grpc itself, but as protobuf is what Tavern officially supports that is how it is checked. Fields which the server did not set are left out of the details, so only the fields actually returned need to be specified even with strict key checking on. The grpc example server now aborts with a google.rpc.Status when given an empty name so there is something real to check against, and the grpc plugin is registered in the unit test conftest so grpc test files can be schema checked there.
The single parametrized test covered three different things at once, with the spec carrying fields which only applied to some of them. Split it into one test per kind of spec: - test_grpc_bad_request: requests which don't match the service definition and never reach the server - test_grpc: requests which run and return the status, error message and details the test expects - test_grpc_bad_response: requests which run, but where the expected response doesn't match what the server returned The failure specs all assert on a substring of the error as well as its type, so they have to fail for the reason they were supposed to. Also renamed that field from 'expected_error' so it isn't confused with the 'error_message' of the response block, and removed 'actual_code' - the status is checked against 'code' in the tests which are expected to pass, and by the response verification itself in the ones which aren't.
…r-handling # Conflicts: # tavern/_plugins/grpc/response.py
Fold _get_rich_details into _verify_details, so that a details parse failure no longer also reports that there were no details. Add a test for details that cannot be decoded, use error_message in the pydantic spec test, and remove a stale TODO.
…entinels error_message takes a string or a str, regex or anything sentinel, and details takes a list or a list or anything sentinel. Test that sentinels are accepted and that other types are rejected.
The status check already fails the test in this case, so this message does not need to be a second error.
…r-handling # Conflicts: # tavern/_core/pydantic_models.py # tests/unit/test_pydantic_models.py
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
… a deprecation warning
change grpc to use 'error_message' for the actual server error message, and 'details' for the rpc details returned in the trailer.
Technically a breaking change(?)Keeping backwqrd compatability for nowOld style, where
detailswas the error message string from the server:New style, where
error_messageis the error message string anddetailsis the list of messages in thegoogle.rpc.Statusfrom thegrpc-status-details-bintrailer:Summary by CodeRabbit